security: Ci/fix validate workflow by qtipbera · Pull Request #392 · berachain/metadata

qtipbera · 2026-05-22T16:29:42Z

This PR splits validate.yml into two workflows to fix a secret exfiltration
vulnerability in the pull_request_target trigger.

The problem

The current validate.yml runs on pull_request_target (which has access to
repo secrets) and checks out fork code into ./head. Any job in that workflow
can read secrets like CLOUDFLARE_ACCOUNT_ID, CLOUDFLARE_IMAGES_API_TOKEN,
and GITHUB_TOKEN. A malicious fork PR could modify validation scripts to
exfiltrate these secrets during CI. The workflow is currently disabled for
this reason.

The fix

validate.yml (pull_request_target, zero secrets):

schema, format, images, coingecko, pyth, data-consistency — all run
against fork code with no secrets in scope
Packages changed asset files as artifacts for the upload workflow
Saves PR metadata (number, head SHA) as artifact

post-validate.yml (workflow_run, has Cloudflare secrets):

Triggers after Validate completes successfully
Downloads asset artifact and PR metadata from the triggering run
Uploads to Cloudflare using base branch scripts only
Never checks out fork code

PR metadata is passed via artifact instead of workflow_run.pull_requests[]
because that array is empty for fork PRs (known GitHub limitation).

Additional fixes

Cache key glob: changed from **/pnpm-lock.yaml to pnpm-lock.yaml.
The ** glob matched lockfiles in subdirectories, meaning a fork could
influence the cache key by adding a lockfile in a nested path.
Unused secrets removed: BERACHAIN_HUB_API_TOKEN and
BERACHAIN_HUB_API_BASE_URL removed from data-consistency. Confirmed
unused via source audit of @berachain/berajs@0.1.0 — getApolloClient
reads config from @berachain/config, not environment variables.
persist-credentials: false added to all fork checkout steps.
head.ref → head.sha on all fork checkouts to prevent TOCTOU race
conditions.

After merge

Re-enable the Validate workflow in the Actions UI.

Copilot

Pull request overview

This PR hardens CI by separating fork-controlled validation work from secret-dependent Cloudflare uploads, mitigating secret exfiltration risks stemming from pull_request_target running untrusted fork code.

Changes:

Split the existing validation pipeline into a no-secrets Validate workflow (runs on pull_request_target) and a secret-bearing Post-Validate workflow (runs on workflow_run).
Pass PR metadata and (optionally) changed assets from Validate → Post-Validate via short-lived artifacts, and remove Cloudflare upload steps from the fork-processing workflow.
Pin GitHub Actions to SHAs, tighten fork checkout behavior (head.sha, persist-credentials: false), and adjust the pnpm cache key to avoid nested-lockfile influence.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
.github/workflows/validate.yml	Removes secret-dependent upload logic, adds artifact packaging for PR info/assets, pins actions, and hardens fork checkout + cache keying.
.github/workflows/post-validate.yml	New workflow_run-driven uploader that downloads artifacts and performs Cloudflare uploads using base-branch scripts with secrets isolated.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

qtipbera · 2026-05-25T15:15:11Z

Note: this PR supersedes #391 (ci/pin-actions-to-sha). Both modify validate.yml — if #391 merges first, this branch will conflict.

This PR is the superset: it includes all of #391's changes (SHA pinning, node24 action upgrades, persist-credentials: false, ref: base.sha on base checkouts) plus the security split, cache key hardening, and unused secret removal. Recommend merging this first and closing #391.

rebasing 392 to main after merge of 391

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: qtipbera <194121515+qtipbera@users.noreply.github.com>

qtipbera · 2026-05-27T11:40:43Z

@bearpong #395 should unblock this PR, but requires admin review and bypass

bearpong · 2026-05-27T14:44:41Z

@qtipbera, #395 merged. Please let's verify this PR works as expected before merging this

resolving conflict with main

qtipbera · 2026-05-27T15:24:24Z

@bearpong All checks green:

schema ✅
format ✅
data-consistency ✅
detect-changes ✅
coingecko ✅
pyth ✅
images ✅
upload-assets ⏭️ (skipped — no asset changes in this PR, expected)
semgrep ✅

This is running against main's workflow (via pull_request_target), which now includes the node24 upgrades (#391), lockfile fix (#393), and setup-node cache fix (#395). The validation ran against fork checkout code in ./head and all jobs passed.

Ready for review.

qtipbera · 2026-06-02T14:35:28Z

Revised — this commit completes the validate/post-validate split.

The earlier version left secrets in the pull_request_target workflow, and post-validate.yml was wired to artifacts that were never produced, so it couldn't actually run. Both are fixed here. Only validate.yml changes; post-validate.yml is unchanged.

`validate.yml`

Deleted the upload-assets job. It carried the Cloudflare credentials + GITHUB_TOKEN in a pull_request_target job that also checks out fork code — the exact exposure this split exists to remove. The Cloudflare upload now lives only in post-validate.yml.
Dropped the Berachain secret from data-consistency. The blocking checks (validateTokens / validateVaults) read the PR's JSON and need no secret, so they're unchanged. The token was only used by validateApiMetadata, which is warnings-only and already skips on failure — validate:data still exits 0 without it. Behavior change to note: the "on-chain item missing metadata" warnings no longer appear on PRs (they were never blocking). If we want them back, they belong in post-validate.yml or a scheduled job — not here.
Added package-pr-context. Checks out the PR head as data only, then publishes the artifacts post-validate.yml consumes: pr-info (PR number / head repo / head sha) and pr-assets (changed files under src/assets/). This is what was missing before.
detect-changes now emits assets-files (list-files: json) so only the files the PR touched get staged.
images → needs: schema (was needs: [schema, upload-assets]); its dependency on the removed job is gone.
Minimal token: top-level permissions: contents: read; removed the per-job pull-requests: write (nothing here writes anymore — commenting happens in post-validate).
Cache key on pnpm-lock.yaml instead of **/pnpm-lock.yaml, so the fork's lockfile can't influence the cache key.
New pin: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 (v5.0.0), matching the download-artifact v5.0.0 already pinned in post-validate.

`post-validate.yml`

Unchanged. It already read PR identity from pr-info (because workflow_run.pull_requests[] is empty for fork PRs), downloaded pr-assets into ./head/src/assets/, and ran the upload from artifacts with no fork checkout. It only failed before because nothing produced those artifacts.

Net result

No secrets anywhere in the pull_request_target workflow; all secret use is isolated in the workflow_run half, which never checks out fork code.

Verification

validate.yml parses, all action refs are SHA-pinned, zero secrets. references.
Staging logic tested against a crafted file list: copies only changed assets, rejects path traversal and non-asset paths.
Confirmed dropping the Berachain token degrades validate:data to a graceful skip, not a failure (the berajs import is safe with no env; the API call no-ops into a "skipping" warning).

The first fork PR after merge is the real end-to-end test (artifact extraction + paths-filter fork-ref resolution). Re-enable the Validate workflow in the Actions UI once this lands.

qtipbera requested review from Timbearlake, bearpong and Copilot May 22, 2026 16:29

Copilot started reviewing on behalf of qtipbera May 22, 2026 16:30 View session

qtipbera added the security label May 22, 2026

Copilot AI reviewed May 22, 2026

View reviewed changes

Comment thread .github/workflows/post-validate.yml Outdated

Comment thread .github/workflows/post-validate.yml

Comment thread .github/workflows/validate.yml Outdated

qtipbera force-pushed the ci/fix-validate-workflow branch from 5dd73d3 to 2072379 Compare May 22, 2026 16:48

qtipbera requested a review from Copilot May 22, 2026 16:48

Copilot started reviewing on behalf of qtipbera May 22, 2026 16:49 View session

This comment was marked as resolved.

Sign in to view

qtipbera requested a review from Copilot May 25, 2026 15:06

Copilot started reviewing on behalf of qtipbera May 25, 2026 15:07 View session

This comment was marked as resolved.

Sign in to view

This was referenced May 25, 2026

security: pin action to sha #391

Merged

fix: regenerate lockfile after dependency bumps #393

Merged

qtipbera and others added 5 commits May 26, 2026 13:24

security: split secrets from fork-processing workflow

881debb

rebasing 392 to main after merge of 391

Potential fix for pull request finding

f7a363e

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: qtipbera <194121515+qtipbera@users.noreply.github.com>

fix: upgrade all actions to node24, pin base checkouts, fix pnpm v2

c303f3f

fix: remove workflow_run.head_sha ref from post-validate checkout

c82e23d

chore: trigger CI

9503eba

qtipbera force-pushed the ci/fix-validate-workflow branch from af5b450 to 9503eba Compare May 26, 2026 17:28

chore: trigger CI

fe2cffd

qtipbera mentioned this pull request May 26, 2026

fix: disable setup-node v5 auto-cache (pnpm not yet installed) #395

Merged

qtipbera added 2 commits May 27, 2026 11:17

fix: disable setup-node v5 auto-cache

2d85abc

Merge remote-tracking branch 'origin/main' into ci/fix-validate-workflow

4c1ebe0

resolving conflict with main

fixing broken validate workflow

60e0ccb

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

security: Ci/fix validate workflow#392

security: Ci/fix validate workflow#392
qtipbera wants to merge 9 commits into
mainfrom
ci/fix-validate-workflow

qtipbera commented May 22, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

qtipbera commented May 25, 2026

Uh oh!

qtipbera commented May 27, 2026

Uh oh!

bearpong commented May 27, 2026

Uh oh!

qtipbera commented May 27, 2026

Uh oh!

qtipbera commented Jun 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

qtipbera commented May 22, 2026

The problem

The fix

Additional fixes

After merge

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

qtipbera commented May 25, 2026

Uh oh!

qtipbera commented May 27, 2026

Uh oh!

bearpong commented May 27, 2026

Uh oh!

qtipbera commented May 27, 2026

Uh oh!

qtipbera commented Jun 2, 2026

validate.yml

post-validate.yml

Net result

Verification

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

`validate.yml`

`post-validate.yml`